[MM] Remove text components from ProcessorInputs - #53093
Conversation
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
DarkLight1337
added
the
ready
|
@claude review |
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
Documentation preview: https://vllm--53093.org.readthedocs.build/en/53093/ |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
@claude review |
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
@claude review |
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
@claude review |
There was a problem hiding this comment.
Beyond the inline nit, I also checked the dummy-input tokenization path in vllm/multimodal/processing/dummy_inputs.py for the profiling-tokenizer-truncation concern raised by finders (the old code explicitly set tokenization_kwargs={"truncation": False}, while the new path uses default_tok_params.get_encode_kwargs()). The default encode kwargs resolve max_length to max_model_len + 1, which is at least as large as any dummy prompt built for profiling at that seq_len, so this doesn't truncate or undercount placeholder tokens in practice.
Extended reasoning...
Reviewed the two dummy-input-tokenization candidate issues from this run's finder/verifier pass (potential truncation/undercounting of placeholder tokens during memory profiling in dummy_inputs.py). Traced default_tok_params.get_encode_kwargs() (vllm/renderers/params.py) and confirmed the resolved max_length is max_model_len + 1, which does not truncate the dummy prompts built for profiling at a given seq_len, matching the verifier's conclusion that this is not a regression.
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
/ci run |
|
✅ Triggered Buildkite CI #84835 for commit |
Isotr0py
left a comment
Isotr0py
left a comment
There was a problem hiding this comment.
Nice, let's make token in / token out now!
| tokenizer = self.info.ctx.tokenizer | ||
| dummy_prompt: list[int] | ||
| if tokenizer is None: | ||
| # Tokenizer-less models (e.g. `skip_tokenizer_init=True`) only | ||
| # accept embeddings and have an empty dummy text, so there are no | ||
| # prompt tokens. | ||
| dummy_prompt = [] | ||
| else: | ||
| dummy_prompt = tokenizer.encode( | ||
| dummy_text, | ||
| **self.info.default_tok_params.get_encode_kwargs(), | ||
| ) |
There was a problem hiding this comment.
Although not in a hurry, should we deprecate get_dummy_text and use token ids for dummy data construction directly in the future and fully avoid text inputs?
There was a problem hiding this comment.
Yes I am planning to do that in the next PR
| def _apply_hf_processor_text_mm( | ||
| self, | ||
| prompt_text: str, | ||
| mm_items: MultiModalDataItems, | ||
| hf_processor_mm_kwargs: Mapping[str, object], | ||
| tokenization_kwargs: Mapping[str, object], | ||
| ) -> tuple[list[int], BatchFeature, bool]: | ||
| ) -> tuple[list[int], BatchFeature]: |
There was a problem hiding this comment.
Actually, I'm reconsidering whether we should rely on hf processor.
Maybe we can just call image processr / video processor / audio processor on demand one by one after token-only inputs refactoring. Then the multimodal processor logic will be much simpler.🤔
There was a problem hiding this comment.
Yeah we could do that as well. That way custom code on HF Hub repos before transformers 5.10 would continue to work. This is also planned for the next PR
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
|
/ci run |
|
✅ Triggered Buildkite CI #84859 for commit |
2 participants
PLEASE FILL IN THE PR DESCRIPTION HERE ENSURING ALL CHECKLIST ITEMS (AT THE BOTTOM) HAVE BEEN CONSIDERED.
Purpose
Follow-up to #53064
Even for the text input path in
AsyncLLM, the prompt is tokenized before being passed to MM processor, so the text path in MM processor is now dead code and can be removed. This PR starts the process by removing textpromptandtok_kwargsfromProcessorInputs.In the next PR I will simplify
_apply_hf_processor_mainby upgrading totransformers>=5.10.4which allows us to pass MM data without any need for dummy text.Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.BEFORE SUBMITTING, PLEASE READ https://docs.vllm.ai/en/latest/contributing (anything written below this line will be removed by GitHub Actions)